Populate dependencies from module graph #109
Conversation
There was a problem hiding this comment.
Awesome, thanks for looking into this!
This relies on an
sbt.internalclass (SbtUpdateReport) which is what is used by the built-in dependency graph/tree plugin; I'm not sure how I feel about that and maybe it'd be better to just re-implement the logic that it uses (https://github.com/sbt/sbt/blob/1.10.x/main/src/main/scala/sbt/internal/graph/backend/SbtUpdateReport.scala) - happy to take views on that.
It looks small enough that re-implementing the logic might be safer - or does it end up 'pulling in' a lot of other internals?
As it makes the generated sbom considerably larger, I think we should make populating the dependencies section optional (default enabled).
| @@ -89,7 +89,7 @@ executed. | |||
| [Scripted](https://www.scala-sbt.org/1.x/docs/Testing-sbt-plugins.html) is a tool that allow you to test sbt plugins. | |||
| For each test it is necessary to create a specially crafted project. These projects are inside src/sbt-test directory. | |||
|
|
|||
| Scripted tests are run using `scripted` command. | |||
| Scripted tests are run using `scripted` command. Note that these fail on JDK 21 due to the old version of sbt. | |||
There was a problem hiding this comment.
should we create an issue for this capturing what exactly is going wrong and what the plan to fix that should be?
There was a problem hiding this comment.
I don't think there's much we can do about it as it's the version of Scala 2.12 pulled in by sbt, so other than moving to a more recent baseline (which feels like a possibly breaking major change) I thought maybe documenting here for local development was sufficient?
There was a problem hiding this comment.
Documenting it is a great improvement. I've opened #110 to discuss going further
Makes sense - I've done both of these. |
README.md
Outdated
| | includeBomToolVersion | Boolean | `true` | include tool version in bom | | ||
| | includeBomHashes | Boolean | `true` | include artifact hashes in bom | | ||
| | enableBomSha3Hashes | Boolean | `true` | enable the generation of sha3 hashes (not available on java 8) | | ||
| | includeBomDependencyTree | Boolean | `true` | include dependency tree in bom (bomSchemaVersion 1.1 and later) | |
There was a problem hiding this comment.
(I'm not sure the detail 'bomSchemaVersion 1.1 and later' is needed here, but OK)
| dependency | ||
| } | ||
|
|
||
| // https://github.com/sbt/sbt/blob/1.10.x/main/src/main/scala/sbt/internal/graph/backend/SbtUpdateReport.scala |
There was a problem hiding this comment.
Hmm. Sorry to go back-and-forth on this a bit, but I just noticed that this is including Apache-licensed code into an MIT-licensed project. That might be possible (they're both premissively-licensed, after all), but we'd want to double-check that adding this comment is sufficient, or that we'd want to do a bit more to make sure this is responsibly tracked.
There was a problem hiding this comment.
That makes sense, thank you for checking this as well
raboof
force-pushed
the
populate-dependencies
branch
from
0d1696c to
969d7fa
Compare
In preparation of including a file with Apache-2.0-licensed code from sbt in sbt#109 https://reuse.software/
raboof
force-pushed
the
populate-dependencies
branch
4 times, most recently
from
5ae3656 to
224f7e2
Compare
There was a problem hiding this comment.
I took the liberty of breaking out the sbt code to its own file (so it can have its own license header).
I proposed a general approach to license headers in #111, but I don't think this PR necessarily needs to be blocked on that. So this PR LGTM (thanks!) but I'll give other maintainers some time to chime in before merging.
|
Thanks so much @raboof, breaking into its own file seems like a pragmatic solution until we can get the SPDX headers in |
raboof
changed the title
In preparation of including a file with Apache-2.0-licensed code from sbt in sbt#109 https://reuse.software/
* Add explicit license headers In preparation of including a file with Apache-2.0-licensed code from sbt in #109 https://reuse.software/ * Add reuse check GitHub action
raboof
force-pushed
the
populate-dependencies
branch
3 times, most recently
from
1c84bed to
e0af721
Compare
raboof
force-pushed
the
populate-dependencies
branch
from
e0af721 to
783b8c4
Compare
…ndencyTree setting (default true)
Since it's licensed under the Apache 2.0 license
raboof
force-pushed
the
populate-dependencies
branch
from
783b8c4 to
d95b03b
Compare
Populates
dependenciesin the bom output by using thedetailsfrom the configuration report.This relies on an
sbt.internalclass (SbtUpdateReport) which is what is used by the built-in dependency graph/tree plugin; I'm not sure how I feel about that and maybe it'd be better to just re-implement the logic that it uses (https://github.com/sbt/sbt/blob/1.10.x/main/src/main/scala/sbt/internal/graph/backend/SbtUpdateReport.scala) - happy to take views on that.Dependencyhas equality based on the ref alone, so calling.distincton the result fromdependenciesForConfigurationis sufficient to ensure that only one ends up in the output.fixes #88